Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add: the start of dart lang support #2671

Merged
merged 13 commits into from
Nov 9, 2024
Merged

Conversation

sonnyvesali
Copy link
Contributor

@sonnyvesali sonnyvesali commented Oct 26, 2024

Hello, hopefully I did these first steps right and I would like some rough guidance on adding the subsequent things like snippets or any other essentials for dart support, thanks!

Checklist

@sonnyvesali sonnyvesali requested a review from a team as a code owner October 26, 2024 21:02
@AndreasArvidsson
Copy link
Member

What you have done here looks right, but you haven't actually added anything. You need to flesh out the scm file for this to be worth merging. Note that you don't need to do the entire language in one pull request, but at least add some useful scopes. Have a look at the other scm files for inspiration.

The Tree sitter query syntax is defined here:
https://tree-sitter.github.io/tree-sitter/using-parsers#query-syntax

The snippet definition have moved to Talon community so you shouldn't really focus on that in Cursorless.

@sonnyvesali
Copy link
Contributor Author

sonnyvesali commented Oct 27, 2024

Hello @AndreasArvidsson, I have added a humble beginning of adding language support starting with the if statement, but for some reason when I run the extension locally, and I have a code snippet like this :final list = [1,2,3] And I say something to cursorless like: if wrap look I still get the language unsupported error, I am probably missing something obvious, any further guidance would be wonderful, thanks.

@AndreasArvidsson
Copy link
Member

AndreasArvidsson commented Oct 27, 2024

Hello @AndreasArvidsson, I have added a humble beginning of adding language support starting with the if statement, but for some reason when I run the extension locally, and I have a code snippet like this :final list = [1,2,3] And I say something to cursorless like: if wrap look I still get the language unsupported error, I am probably missing something obvious, any further guidance would be wonderful, thanks.

The snippets have nothing to do with the language definition in the scm files. The snippets are a separate system that is being deprecated in favor of the community .snippet files.

First you need a language definition in vscode for dart. ie you need to install a dart extension so that vscode correctly recognizes the file as the language dart. You can see in the bottom right corner which language vscode has associated with the current document.

Once that is done and you have added a scope in their scm file for example if statement you should be able to say. "take if state" in dart and have the if statement be selected.

@pokey
Copy link
Member

pokey commented Oct 30, 2024

You'll need to run pnpm -w fix:meta to fix that CI bug. It will generate a stub language doc page. We currently hide them but eventually will expose them in the docs

After that is fixed you're going to get an error saying you're missing a test for the if statement scope. See steps 4 and 6 of the docs for adding a new language

@pokey
Copy link
Member

pokey commented Nov 1, 2024

almost there! looks like you've forgotten to run step 6 of the docs I believe. But possible you already ran that and something is broken

have you tested the scope manually either using scope visualiser or "take if state"?

@sonnyvesali
Copy link
Contributor Author

@pokey Yes I have tested with take if state, and it works. Ran debug edit subset & update fixtures subset in both the main project and a sandboxed VS Code instance, but nothing happened.

Side question: I’m running into errors with query code copied from JavaScript, even though Dart has identical syntax in terms of arrays and maps (called lists in dart not sure if that detail matters). Errors like Expected language definition entry is missing for languageId dart and RangeError: Bad node name 'object' appear when I try it. Any insight on adapting this for Dart?

Query code:


;;!! if () {}
;;!  ^^^^^^^^
(if_statement) @ifStatement

;;!! { value: 0 }
;;!  ^^^^^^^^^^^^
[
  (object)
  (object_pattern)
] @map

;;!! [ 0 ]
;;!  ^^^^^
[
  (array)
  (array_pattern)
] @list

Language scope code:

import type { LanguageScopeSupportFacetMap } from "./scopeSupportFacets.types";
import { ScopeSupportFacetLevel } from "./scopeSupportFacets.types";

const { supported, unsupported, notApplicable } = ScopeSupportFacetLevel;

export const dartScopeSupport: LanguageScopeSupportFacetMap = {
  list: supported,
  map: supported,
  ifStatement: supported,
};

@pokey
Copy link
Member

pokey commented Nov 2, 2024

@pokey Yes I have tested with take if state, and it works. Ran debug edit subset & update fixtures subset in both the main project and a sandboxed VS Code instance, but nothing happened.

Ah right yeah those docs are kind of missing a step. You need to ensure that the subset of tests you're running is the dart tests, if you've not done that already. See test subset doc for more

Side question: I’m running into errors with query code copied from JavaScript, even though Dart has identical syntax in terms of arrays and maps (called lists in dart not sure if that detail matters). Errors like Expected language definition entry is missing for languageId dart and RangeError: Bad node name 'object' appear when I try it. Any insight on adapting this for Dart?

Is object a valid node type in the dart tree-sitter grammar?

@sonnyvesali
Copy link
Contributor Author

@pokey sorry to sound like a noob, but how can I figure out if object is valid in the dart tree sitter grammar do I look at the grammar.json in the npm tree-sitter-dart library?

If that is the case, when I scaffold the file and look for object mentions, I see some mentions of it, but not sure if it relates link to grammar.json to check my analysis.

Thanks!

@pokey
Copy link
Member

pokey commented Nov 2, 2024

See the tips. In particular, if you make a file with a map in it and say "parse tree file" you will see the parse tree. And fwiw here is the tree-sitter grammar for dart

@sonnyvesali
Copy link
Contributor Author

Hi, I'm having trouble recording tests and can’t figure out what I might be doing wrong. I've added what I believe is the correct fixture for the single if statement scope, but I'm unsure how test updating is supposed to work. Here’s what I’m doing:

  1. I use the command "debug edit subset" and modify testSubsetGrep.properties like this:

    # This file contains grep strings for Mocha when running a subset of tests.
    # These will be used with the "Run test subset" launch configuration.
    # See https://mochajs.org/#-grep-regexp-g-regexp for supported syntax.
    #
    # One regular expression per line.
    # Tests matching any of the regular expressions will be run.
    
    languages/dart/`
    
  2. I then say Update fixtures subset and launch the tests.

I’m not sure what the test recorder should be doing here, and it doesn’t seem to be working as expected. If the only file that's supposed to be changed is the fixtures file, why are the tests still failing? Any guidance would be helpful I am so lost.

@pokey
Copy link
Member

pokey commented Nov 3, 2024

Replace that whole testSubsetGrep file with a single line containing dart

If that voice command isn't working try saying "bar run" to pull up the debugging sidebar and in the top dropdown select VSCode: Update fixtures (subset) (or something like that) and hit the play button

@sonnyvesali
Copy link
Contributor Author

sonnyvesali commented Nov 3, 2024

Thank you! Tests passed in ci, time to do the first merge? 😁 Or should I add more scopes?

@pokey
Copy link
Member

pokey commented Nov 3, 2024

That's curious. What happens when you pull up if(true) { } in a dart file and say "visualize if state"? My conjecture is that nothing will get highlighted. Can you try running "parse tree file" on that same file and paste the results here?

@sonnyvesali
Copy link
Contributor Author

output for visualize if state:
Scope type not supported for dart, or only defined using legacy API which doesn't support visualization. See https://www.cursorless.org/docs/contributing/adding-a-new-language/ for more about how to upgrade your language.

output for parse tree file:

Cursorless parse tree

dart.dart [0:0-5:0]

hi() {
  if (true) {
    return true;
  }
}
(program
  (function_signature
    name: (identifier)
    (formal_parameter_list
      "("
      ")"
    )
  )
  (function_body
    (block
      "{"
      (if_statement
        "if"
        "("
        (true
          "true"
        )
        ")"
        consequence: (block
          "{"
          (return_statement
            "return"
            (true
              "true"
            )
            ";"
          )
          "}"
        )
      )
      "}"
    )
  )
)
program [0:0-5:0]
  function_signature [0:0-0:4]
    name: identifier [0:0-0:2]
    formal_parameter_list [0:2-0:4]
      "(" [0:2-0:3]
      ")" [0:3-0:4]
  function_body [0:5-4:1]
    block [0:5-4:1]
      "{" [0:5-0:6]
      if_statement [1:2-3:3]
        "if" [1:2-1:4]
        "(" [1:5-1:6]
        true [1:6-1:10]
          "true" [1:6-1:10]
        ")" [1:10-1:11]
        consequence: block [1:12-3:3]
          "{" [1:12-1:13]
          return_statement [2:4-2:16]
            "return" [2:4-2:10]
            true [2:11-2:15]
              "true" [2:11-2:15]
            ";" [2:15-2:16]
          "}" [3:2-3:3]
      "}" [4:0-4:1]

@pokey
Copy link
Member

pokey commented Nov 3, 2024

Wait so "take if state" works but "visualize if state" doesn't?

@pokey
Copy link
Member

pokey commented Nov 3, 2024

It might be helpful if you say "bar cursorless" with that file open and send a screenshot

@pokey
Copy link
Member

pokey commented Nov 3, 2024

Just to be sure: you're running "visualize if state" from the debug vscode instance, not the host instance?

@sonnyvesali
Copy link
Contributor Author

oops my bad here is a screenshot of 'visualize if state'
Screenshot 2024-11-04 at 7 19 24 AM
here is also 'bar cursorless':
Screenshot 2024-11-04 at 7 19 44 AM

based on my impressions this appears to be working fine

@pokey
Copy link
Member

pokey commented Nov 5, 2024

ok cool. what happens if you use exactly the file

if(true) { }

like in your test case?

@sonnyvesali
Copy link
Contributor Author

sonnyvesali commented Nov 6, 2024

image

I believe the same thing as before it would appear that this fully works, I said 'take if state'

@pokey
Copy link
Member

pokey commented Nov 6, 2024

That file is different from the one in your test case. Notice that it is 3 lines but your test case is only 1 line

@sonnyvesali
Copy link
Contributor Author

modified the ifStatement scope to be valid code, thanks for the pointers

@sonnyvesali
Copy link
Contributor Author

Hi what am I missing from my test fixtures exactly is there something I need to do regarding cursor placement that can be specified in the fixture??

@pokey
Copy link
Member

pokey commented Nov 8, 2024

you just need to re-run update fixtures and you should be good to go

@sonnyvesali
Copy link
Contributor Author

sonnyvesali commented Nov 8, 2024

@pokey the fixtures are already valid dart code with an if statement, I don't understand what needs to be updated, or rather why the tests were failing it's a valid if block & it works 'take if state' in the debug instance, what is there to update

@sonnyvesali
Copy link
Contributor Author

sonnyvesali commented Nov 8, 2024

when I look at the other fixtures they have these content in domain things in them and I don't know how you get those like this in javascript for the if statement:

if (true) {

}
---

[Content] =
[Removal] =
[Domain] = 0:0-2:1
  >-----------
0| if (true) {
1|
2| }
   -<

[Insertion delimiter] = "\n"

so very clearly I'm missing something in the way that I'm constructing these tests because these are not showing up.

@pokey
Copy link
Member

pokey commented Nov 9, 2024

Have you tried re-running update fixtures?

@pokey
Copy link
Member

pokey commented Nov 9, 2024

Sorry to be so opaque. Basically these tests have two sections. The section above the --- is the dart code, and the section below indicates which scopes were found in the code. The fact that it was empty indicates that cursorless found no scopes (if statements in this case) in the code. The fact that CI was passing before meant that there were no if statements found in the code

When you fixed the dart code, notice that CI started failing, because the fixture says there should be no scopes but now it finds one

Running update fixtures will replace the section below the --- with the value that it should be. Before, there were no scopes, so it just left it blank. Now, when you run it, it should add the scopes

Make sense?

@sonnyvesali
Copy link
Contributor Author

I think I understand, that lower section updated, thank you for your patience

@sonnyvesali
Copy link
Contributor Author

sonnyvesali commented Nov 9, 2024

I think I'm getting the hang of this! I added the list and map scopes I think this should be enough for an initial PR

Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have lift-off! 🚀🙌

@pokey pokey enabled auto-merge November 9, 2024 16:13
@pokey pokey added this pull request to the merge queue Nov 9, 2024
Merged via the queue into cursorless-dev:main with commit 762f377 Nov 9, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants